ENH: update bundled ujson to latest v1.33 #3946
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Komnomnomnom can you hook up to travis? (then recommit this) |
|
@cpcloud can you give this a run on mac? |
|
wish i could. i only have a black macbook which has arch linux on it and i don't think can be upgraded to a version of os x that uses clang by default...maybe one of @wesm, @rhstanton, @fonnesbeck, or @selasley? |
|
the existing ujson in master compiles and tests completey fine on amd64 (I use msvc10) but this release fails several tests, then crashes the interpreter |
|
sorry...the build actually crashed |
|
I can try it out on a Mac if you walk me through how to checkout this branch and ensure that it builds with clang. |
|
u might need to change |
|
u should probably use hub but see below for the "quick-and-dirty" solution. # do this somewhere other than where you've cloned pandas master
git clone git://github.com/Komnomnomnom/pandas
git checkout ujson-update
python setup.py build_ext --inplace |
|
Hmm worked fine for me before I sent the pull request (64-bit Arch Linux), I'll take a look |
|
i can confirm that this builds fine on arch linux 64 as well |
|
Here's a gist for the build. Running the tests now. |
|
Looks like it's VC++ being bit of a stickler again for having variables declared at the beginning of functions. Should be an easy fix. |
|
Should I be using tox for the tests? I just started reading into it a bit. |
|
looks like u may have discovered a few bugs |
|
rpy tests should skip, not sure about the datetime or plot tests (both pass on my machine) |
|
The JSON tests passed though, thanks @TomAugspurger ! |
|
@jreback took a while but travis ci finally kicked in and looks ok. Can you try again with msvc (if not already covered by travis)? |
|
@Komnomnomnom built on amd64 windows just fine (and tests al pass) looks ready to merge? |
|
@jreback sounds good! As long as @cpcloud and @TomAugspurger are happy with the clang build? |
|
@TomAugspurger can you give another run with latest of this PR, just to make sure.....thxs |
|
i'm ok with this, but that doesn't really mean a whole lot since i can't test with a mac. arch linux build is fine as i said b4... |
|
@jreback that looks interesting i guess only objective-c is allowed since that is all that exists in appleland |
|
I look forward to the iOS port of pandas ;)
|
|
after I read it I realized mac is not so soon (and windows even farther off)..... |
|
no love for propriety from travis |
|
Here's the relevant part of the build output for the json module, at least I think this is all of it. I can post the whole thing if need be. Should I rerun the tests? |
|
@TomAugspurger did it work? Otherwise I can test on my mac. |
|
Tests looked good on the newest commit. Same as my previous one. |
|
@jreback ok with this? |
|
yep go ahead |
cpcloud
added a commit
that referenced
this pull request
Jun 19, 2013
ENH: update bundled ujson to latest v1.33
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates pandas JSON to use the latest ujson version. A fair few fixes and enhancements are included. Unfortunately however there were a lot of whitespace changes so it looks like a lot more than it actually is.
All tests pass on py27 and py33. Valgrind run of JSON tests with Python 2.7 is clean.
Also included are two fixes encountered during merging and pushed upstream to ultrajson:
ultrajson/ultrajson#93
ultrajson/ultrajson#94